sample(app): add Firestore, Database & Storage demos to sample app#2406
sample(app): add Firestore, Database & Storage demos to sample app#2406demolaf wants to merge 21 commits into
Conversation
…stUtils in paging test
…instance and add unit tests
…2383) * fix(auth): validate display name only when required in sign-up form * fix(auth): use locale-safe matcher for sign-up button in test
# Conflicts: # app/src/main/AndroidManifest.xml # app/src/main/java/com/firebaseui/android/demo/MainActivity.kt # app/src/main/java/com/firebaseui/android/demo/auth/HighLevelApiDemoActivity.kt # storage/build.gradle.kts
There was a problem hiding this comment.
Code Review
This pull request restructures the demo application by modularizing the Auth demos and introducing new demonstration activities for Firebase Realtime Database, Firestore, and Storage (using Glide). It also updates FirestorePagingSource to properly handle InterruptedException and avoid undeliverable RxJava exceptions. Feedback focuses on several key improvements: configuring and initializing the Realtime Database emulator, exposing and passing the emulator mode to the Auth chooser screen, resolving a bug in the Storage demo where Glide loads are triggered on every keystroke, programmatically converting dp to pixels for view holder padding to avoid raw pixel anti-patterns, and optimizing Firestore data seeding using a WriteBatch.
d382bd5 to
1ecd498
Compare
russellwheatley
left a comment
There was a problem hiding this comment.
A few suggestions and nits on the demo additions below.
This branch also carries an unrelated commit (d382bd5, the sign-up display name validation fix) that doesn't belong here, likely from being stacked on another branch. Worth rebasing it out before merge, same ask as on the edge-to-edge insets PR.
| var pendingEmailLink = intent.getStringExtra(EmailLinkConstants.EXTRA_EMAIL_LINK) | ||
| if (USE_FIRESTORE_EMULATOR) { | ||
| FirebaseFirestore.getInstance() | ||
| .useEmulator(FIRESTORE_EMULATOR_HOST, FIRESTORE_EMULATOR_PORT) |
There was a problem hiding this comment.
This runs on every onCreate(), including activity recreation (e.g. rotation), but useEmulator() can only be called once per FirebaseFirestore/FirebaseDatabase instance before first use, so a second call throws IllegalStateException. Worth guarding this with a one-time flag or moving it to Application.onCreate().
|
|
||
| if (USE_DATABASE_EMULATOR) { | ||
| FirebaseDatabase.getInstance() | ||
| .useEmulator(DATABASE_EMULATOR_HOST, DATABASE_EMULATOR_PORT) |
There was a problem hiding this comment.
Same issue here for the Database emulator call.
|
|
||
| import android.os.Bundle | ||
| import android.util.Log | ||
| import com.firebaseui.android.demo.R |
There was a problem hiding this comment.
This import landed mid-block instead of alphabetically. Minor, but worth fixing if there's a lint/spotless pass anyway.
| import com.firebase.ui.auth.ui.screens.AuthSuccessUiContext | ||
| import com.firebase.ui.auth.ui.screens.FirebaseAuthScreen | ||
| import com.firebase.ui.auth.util.EmailLinkConstants | ||
| import com.firebaseui.android.demo.R |
There was a problem hiding this comment.
Same unsorted import here.
|
|
||
| <activity | ||
| android:name=".HighLevelApiDemoActivity" | ||
| android:name="com.firebaseui.android.demo.auth.HighLevelApiDemoActivity" |
There was a problem hiding this comment.
This uses the fully-qualified activity name while the other moved auth activities (e.g. .auth.CredentialLinkingDemoActivity below) use the shorter relative style. Worth picking one convention for consistency.
Adds demo activities for Realtime Database, Firestore, and Storage to the sample app.
auth/subpackage; adddatabase/,firestore/,storage/subpackagesDatabaseDemoActivity— paginated list viaFirebaseRecyclerPagingAdapter+orderByChildFirestoreDemoActivity— paginated list viaFirestorePagingAdapter+orderBy, seeded with aWriteBatchStorageDemoActivity— loadsgs://URLs via Glide, both as a raw string (StringLoader) and viaStorageReferenceMainActivityPreview
sample-demo.webm